add internal git ReadAPI/WriteAPI impl scaffolding + refactor GitBlobstore reads#10414
add internal git ReadAPI/WriteAPI impl scaffolding + refactor GitBlobstore reads#10414coffeegoddd merged 4 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the Git-backed blobstore implementation by introducing a principled layering between the high-level GitBlobstore and low-level git plumbing operations. It adds a WriteAPI implementation (scaffolding for future use) and extracts the existing read functionality into a ReadAPI interface with a concrete implementation.
Changes:
- Introduced ReadAPI interface and ReadAPIImpl to encapsulate git read operations
- Added WriteAPI implementation (WriteAPIImpl) with plumbing commands for future write support
- Extracted OID type definition to a separate file for better organization
- Refactored GitBlobstore to use ReadAPI instead of calling git functions directly
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| go/store/blobstore/internal/git/write_impl.go | New file implementing WriteAPI with git CLI plumbing commands for write operations (read-tree, update-index, write-tree, commit-tree, update-ref) |
| go/store/blobstore/internal/git/util.go | New file with utility functions ReadAllBytes and NormalizeGitPlumbingError |
| go/store/blobstore/internal/git/read_impl.go | New file implementing ReadAPI with all read operations previously in read.go |
| go/store/blobstore/internal/git/read.go | Refactored to define ReadAPI interface instead of standalone functions |
| go/store/blobstore/internal/git/oid.go | New file extracting OID type definition |
| go/store/blobstore/git_blobstore.go | Updated to use ReadAPI interface methods instead of standalone functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…api, refactor reads a bit
coffeegoddd
force-pushed
the
db/gitblobstore-next
branch
from
70f4dbc to
aa59970
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func ReadAllBytes(ctx context.Context, r *Runner, oid OID) ([]byte, error) { | ||
| rc, err := NewAPIImpl(r).BlobReader(ctx, oid) |
There was a problem hiding this comment.
The ReadAllBytes function creates a new APIImpl instance on every call. This could be inefficient if called frequently, as it allocates a new struct each time. Consider accepting an existing GitAPI interface parameter instead of creating a new APIImpl, or accepting a pre-existing APIImpl. This would be more consistent with how other parts of the codebase use the API (e.g., GitBlobstore stores an api field).
|
@coffeegoddd DOLT
|
This PR advances the Git-backed blobstore.Blobstore prototype by adding a principled internal git write plumbing layer (still unused by GitBlobstore for now) and refactoring read plumbing into a
ReadAPIinterface + concrete impl to match the write side.